Skip to content

mgmt, upgrade features RP#23211

Merged
weidongxu-microsoft merged 6 commits intoAzure:mainfrom
weidongxu-microsoft:mgmt_upgrade-feature
Jul 29, 2021
Merged

mgmt, upgrade features RP#23211
weidongxu-microsoft merged 6 commits intoAzure:mainfrom
weidongxu-microsoft:mgmt_upgrade-feature

Conversation

@weidongxu-microsoft
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft commented Jul 28, 2021

tests pass under resources

basically, only this is manual change fae32ca

@weidongxu-microsoft weidongxu-microsoft marked this pull request as ready for review July 28, 2021 07:44
Comment on lines +20 to +24
features.stream()
.filter(f -> "NotRegistered".equals(f.state()))
.findFirst()
.ifPresent(feature -> resourceClient.features()
.register(feature.resourceProviderName(), feature.featureName()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think find a Registered one is better (i think it could be registered). Otherwise when all providers are registered, the test can not test anything.

Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now most feature is NotRegistered (and we got lots of them for this test to run, we can change that after we exhausted them).

BTW, if I find Registered, I cannot call unregister on it as someone may depends on that feature.

@weidongxu-microsoft
Copy link
Member Author

/check-enforcer override

@weidongxu-microsoft
Copy link
Member Author

all CI pass, just check-enforcer not done

@weidongxu-microsoft weidongxu-microsoft merged commit 96065c4 into Azure:main Jul 29, 2021
@weidongxu-microsoft weidongxu-microsoft deleted the mgmt_upgrade-feature branch July 29, 2021 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants